Review 汇总（Claude Code + Codex 双人 review）

整体方向认可：fire-and-forget 兜底干净、默认不配 hooks 时零行为变化、向后兼容、tsc --noEmit 通过、25 个 hook 相关测试全过。但有 3 个问题建议合并前处理，外加 3 个非阻塞项。

必须修（合并前）

P1 ｜ hook 子进程生命周期：timeout 不杀进程树，CLI 会被挂住
src/services/hook-runner.ts:246-293：spawn 后没有 child.unref()；超时处理只 child.kill('SIGTERM') 直接子进程，且不主动 settle，依赖 close 事件——但 Node 的 close 要等 stderr pipe 关闭。

• 因为 child handle 没 unref，botmux send 这种临时 CLI 进程会一直等到 hook 跑完/超时才退出（实测 outbound hook 配 /bin/sleep 1 → 退出耗时 1.14s；默认 5s 超时 → ~5.5s）。
• 更严重：bash wrapper hook（本 PR 自带的示例脚本都是 bash）拉起的长跑孙进程会继续占着 stderr pipe，close 永不触发 → promise 永不 settle → botmux send 无限挂住（实测需手动 kill 孙进程才解除）。

建议：detached: true + 超时时杀整个进程组（process.kill(-pid, 'SIGTERM/SIGKILL')），并在 timeout 时主动 settle 而不只依赖 close；同时明确 CLI 上下文到底"等超时"还是 unref 不等。

P2 ｜ hook 继承完整环境，泄漏机密
src/services/hook-runner.ts:249-252 把 ...process.env 原样传给 hook 子进程。botmux worker 明确注入 LARK_APP_SECRET，CLI 链路把 worker env 传给 agent，因此 botmux send 触发的 outbound hook 至少能拿到 Lark secret，也可能带出 GITHUB_TOKEN/OPENAI_API_KEY 等 ambient secret。建议默认改最小化/allowlist env；若保留可配置继承，需在文档里写清 trust boundary。

P2 ｜ quote-reply 路径漏传 hookContext，功能在主路径上静默失效
src/cli.ts 里 hookContext 只接到了 dispatch()（仅用于文件附件/非引用路径），但普通群发正文/卡片走的是 dispatchPrimary() 的引用分支：src/cli.ts:2729 的 replyMessage(appId, quoteTargetId, ..., false) 和 :2736 撤回 fallback 的 sendMessage(...) 都没带 hookContext。结果普通群默认发送路径上，outbound.reply/send payload 缺 sessionId/chatId/rootMessageId/title——即本 PR 主打的"outbound payload 带 sessionId"在最常见路径上没生效。test/cli-send-hook-context.test.ts 只 grep 了 dispatch 那两行，没覆盖这条默认路径。

建议修（非阻塞 P3）

• lastIdleEmits Map 无界增长：src/services/session-lifecycle-hooks.ts:15,63-67 每个 session 留 2 个 dedup key，会话结束后不清理。建议 session.exit 时按 sessionId prune。
• redaction 不递归：CONTENT_FIELDS（hook-runner.ts:57-59）只截断顶层 content/message/description/finalOutput/lastScreenContent；session.requires_attention 的 optionsPreview[].text/label（worker-pool.ts tui_prompt 分支）原样进 payload，绕过 600 字符截断，与"内容默认截断"文档不一致。
• 失败日志缺 hook command/index：hook-runner.ts:311 只打了 event 名，配多个同事件 hook 时难定位是哪条挂了。

已排除

每次 emit 的 existsSync/statSync 开销：基准测试 10 万次，无 hooks 文件 1.15µs/次、有缓存 1.88µs/次，非问题。

合并前验收口径

1. botmux send 不再因 outbound hook 等 hook 完成或等默认 timeout 才退出；bash wrapper 拉起长跑孙进程也不能挂住 CLI。
2. timeout 主动 settle + 清理进程组；测试覆盖 bash wrapper + orphan/pipe 场景。
3. hook env 默认最小化/allowlist，至少不继承 LARK_APP_SECRET/GITHUB_TOKEN/OPENAI_API_KEY；保留可配置继承则文档写清 trust boundary。
4. quote reply 和 withdrawn fallback 两条都补 hookContext，测试直接 mock replyMessage/sendMessage 验参数，不只 grep 源码。

@Lotu527 想确认一下你近期是否有时间处理上面的 P1 + 两个 P2（P3 可顺手但别扩大改动面）？如果你最近排不开，我们这边可以接手按上述验收口径来修、改完会再做一轮 focused re-review。你方便的话回个话 🙏

@deepcoldy OK，今天我处理下

重新 review 后续（5 个新 commit 之后）

感谢快速响应，先放结论：上一轮 6 个 finding 我们逐条核了源码，P1 子进程生命周期、P2 env 泄漏、P2 quote hookContext、P3 lastIdleEmits 清理、P3 失败日志改进——全部修得很扎实，env allowlist 经实测注入 LARK_APP_SECRET/GITHUB_TOKEN/OPENAI_API_KEY 都不会传给 hook，quote 路径的 dispatchPrimaryMessage 抽得很干净也加了真实 mock 测试。tsc --noEmit 通过、31 个 hook 相关测试通过。

不过新一轮 review 又冒出 3 个点，1 个是已修 P1 的边角余量、2 个是这次新引入的，建议合并前都处理一下。

1. 🔴 session.error 是死代码（commit dbcd0b3）

commit message 写的是"worker.ts: add PTY error scanner"，但实际 git show --name-only dbcd0b3 只列了 4 个文件：

src/core/worker-pool.ts
src/services/hook-runner.ts
src/services/session-lifecycle-hooks.ts
src/types.ts

src/worker.ts 不在里面。全仓 rg pty_error 也只在类型定义（types.ts:198）、消费方（worker-pool.ts:1369-1377 的 case）和 emit helper（session-lifecycle-hooks.ts:84-89）出现——没有任何地方 send 这条 IPC。

后果：worker-pool.ts 里那个 case 'pty_error' 收不到消息（dead code），emitSessionErrorHook 永远不会被调用，session.error hook 永远不会触发。八成是 git add src/worker.ts 漏了。

建议二选一：把 worker.ts 的 PTY 扫描器补进来；或者把 session.error 整套（type / case / helper / HOOK_EVENTS 那一行）从本 PR 摘出，单独开 PR。

2. 🟠 嵌套 redaction 字段名打错，等于没截

prepareHookPayload 截的是 payload['options']：

// src/services/hook-runner.ts:174-188
const optionsValue = payload['options'];
if (Array.isArray(optionsValue)) {
  payload['options'] = optionsValue.map(item => { /* truncate text/label */ });
}

但 session.requires_attention 实际 emit 的字段是 optionsPreview：

// src/core/worker-pool.ts:1221
optionsPreview: msg.options.slice(0, 5).map(option => ({
  text: option.text,
  label: option.label,
  // ...
})),

所以 optionsPreview[].text/label 还是绕过 600 字符限制——原 P3 等于没修。

新增的测试 truncates long text/label in nested options array 构造的 payload 字段叫 options、断言也查 options，所以测试是绿的，但跟真实 emit 路径完全不沾。

修法：要么把 redaction 改成扫 options + optionsPreview 两个字段；要么直接把 worker-pool 那行字段名改成 options（往后再加新嵌套 payload 也走同样字段约定）；测试同时改成断言 optionsPreview。

3. 🟡 P1 余量：CLI 路径下 timeout 不再执行，长跑 hook 会变孤儿进程

这条是上一轮 P1 修复留下的副作用。机制：

• fireAndForget 模式下，hook-runner 把 timer.unref() 了（src/services/hook-runner.ts:329 左右）；
• outbound hook 是在消息发送 成功之后 才启动；
• 短命的 botmux send 进程很快就退出，unref 的 timer 没机会触发；
• 于是 timeout 回调里的 process.kill(-pid, 'SIGTERM/SIGKILL') 进程组清理 不会执行；
• 由于 child 是 detached: true + unref()，它会变成永久后台孤儿进程。

实测复现（Codex）：bash wrapper hook 拉起长跑 node 孙进程，CLI 1.77s 正常退出，500ms 后孙进程仍存活；timeoutMs:100 也一样，因为 timer 在 CLI 退出后才"该触发"，但 CLI 已经没了。对比：daemon 路径（timer 不 unref）下 process.kill(-pid) 工作正常，100ms 超时后孤儿数=0。

也就是说，README/事件表说的"超时后会先 SIGTERM 再 SIGKILL"在 botmux send 场景下并不成立——CLI 不挂了，但"超时杀手"也跟着失效了。两者得调和。

推荐方向：让 daemon 当 CLI hook 的监工

botmux send 触发 outbound hook 时，不在 CLI 进程里 spawn，而是 HTTP POST 一下 daemon 的本机 IPC 端口（复用 /api/asks 的鉴权 / 端口发现逻辑），把 {event, payload, hookContext} 提交给 daemon 就立刻返回。daemon 收到后走现有的 emitHookEvent——daemon 模式下保持 timer 不 unref，所以 5s 超时、进程组 kill 都照常生效。

收益：

• botmux send 退出速度只多一次本地 HTTP（毫秒级），不感知；
• 超时承诺重新兑现，跑飞的脚本不会留孤儿；
• 架构上"谁拉起 hook"统一收敛到 daemon 一家（inbound / scheduler / lifecycle 本来就在 daemon 里）；
• daemon 不可达时 warn 跳过即可，hook 本身就是 best-effort。

实现轮廓（仅供参考）：

• hook-runner 暴露一个 runOrForwardHook(event, payload)；
• CLI 进程里通过 findDaemon(larkAppId) + fetch('/api/hooks/emit', POST) 转发；
• daemon 注册 /api/hooks/emit 接口直接调 emitHookEvent（fireAndForget 留 false）；
• 现有 daemon 侧调用点完全不动。

如果觉得这一刀切太大，自包含的备选是"CLI 拉一个迷你 detached supervisor 来跑 timer"，但每个 hook 多一个常驻进程，没那么干净。

合并前验收口径（更新）

1. session.error 要么补 worker.ts 扫描器（含测试覆盖一条 PTY 错误样本走完 scanner → IPC → emitSessionErrorHook → Lark reply 全链路），要么从本 PR 摘掉。
2. 嵌套 redaction 真正覆盖 optionsPreview（无论改截哪个字段名都行），测试断言改成查真实 emit 字段，最好直接拿 worker-pool 跑出来的 payload 走一遍。
3. CLI 路径下"超时承诺"恢复成立（推荐 daemon-supervised 路线），测试要覆盖：
   • botmux send 在 hook 卡住时仍能毫秒级退出；
   • 同时 daemon 侧 timeout 到点能 SIGTERM/SIGKILL 进程组，孤儿孙进程也被回收。

修完我们再做最后一轮 focused re-review。需要讨论实现细节随时 @ 我们 🙏

— 双人 review (Claude Code + Codex)

@deepcoldy 近期事情有点忙可能来不处理，你们可以先接手处理下

@Lotu527 好的，没问题，我抽空试着改改看

Merged 🎉 感谢 @Lotu527 这一轮的快速迭代，前两轮 review 提的 6 个 finding 都被你扎实处理掉了。最后一轮我们以 reviewer 身份把 3 个残留也整合进来：

• session.error 死代码 revert 掉了（缺 worker.ts PTY 扫描器，后续单独 PR 更合理）
• 嵌套 redaction 字段名 改成同时识别 optionsPreview / options
• CLI fireAndForget 孤儿 通过 daemon-supervised forwarding 根治：CLI 路径 emit 转发到 daemon 的新 /api/hooks/emit，由长寿命 daemon 负责 timeout + 进程组 kill；botmux send 仍毫秒级退出

完整 dual review 走完：Codex 独立 PASS（含 daemon-supervised harness 端到端实测：CLI 205ms 退出 / timeout 触发 / 800ms 后 survivor=0）；tsc clean；33 hook tests 全过。

merge commit: da90722，未发版（待 deepcoldy 决定）。后续 PTY scanner 那块期待你开新 PR，我们继续配合 review 🙏